Skip to content

Playwright visual regression - #345

Merged
ritik4ever merged 5 commits into
ritik4ever:mainfrom
EmeditWeb:playwright-visual-regression
Jun 2, 2026
Merged

Playwright visual regression#345
ritik4ever merged 5 commits into
ritik4ever:mainfrom
EmeditWeb:playwright-visual-regression

Conversation

@EmeditWeb

@EmeditWeb EmeditWeb commented May 28, 2026

Copy link
Copy Markdown
Contributor

Close #315

Summary

I Added Playwright visual regression coverage for the CampaignCard component.

What changed

  • Added e2e/campaign-card.visual.spec.ts to render CampaignCard in four states: open, funded, claimed, failed
  • Added playwright.visual.config.ts to configure screenshot storage under e2e/screenshots/
  • Set maxDiffPixelRatio: 0.005 for Playwright screenshot assertions
  • Added npm run test:visual script
  • Added GitHub Actions workflow .github/workflows/playwright-visual-regression.yml to run visual tests and upload artifacts on failure

Testing

  1. Run npm install from repo root
  2. Run npm run test:visual
  3. Verify Playwright launches the frontend and captures screenshots for all four campaign card states
  4. Confirm baseline screenshots are generated under e2e/screenshots/
  5. Confirm the test fails if pixel diff exceeds 0.5%
  6. Verify CI artifact upload is configured by checking the new workflow file

Notes

This adds regression coverage for layout, progress bar rendering, badge state, and color styling in the CampaignCard component.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added automated visual regression testing for campaign components.
    • Implemented theme mode persistence and selection.
  • Bug Fixes

    • Improved error handling with better toast messages for user actions.
    • Enhanced CORS error responses and campaign selection initialization.
  • Documentation

    • Significantly expanded deployment guide with detailed platform-specific instructions for contract, backend, and frontend deployment.
  • Chores

    • Added code formatting and linting configuration.
    • Updated development scripts for improved workflow.

@vercel

vercel Bot commented May 28, 2026

Copy link
Copy Markdown

@EmeditWeb is attempting to deploy a commit to the ritik4ever's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces Playwright visual regression testing infrastructure and deployment documentation, major frontend App refactoring with visual test mode and improved UX flows, backend CORS error handling, and repository-wide Prettier formatting setup. Visual tests validate CampaignCard across all states.

Changes

Visual Regression Testing, Deployment Docs, and Application Updates

Layer / File(s) Summary
Playwright visual regression testing infrastructure
.github/workflows/playwright-visual-regression.yml, playwright.config.ts, playwright.visual.config.ts, e2e/campaign-card.visual.spec.ts
GitHub Actions workflow runs visual tests on pull requests; Playwright configs set screenshot output paths and visual-only test settings; E2E test iterates campaign states and asserts screenshot matches with pixel diff tolerance.
Deployment documentation expansion
DEPLOYMENT.md
Comprehensive rewrite with prerequisites, step-by-step Soroban contract deployment (including contracts/contract_id.txt setup), Render backend build/start commands and health check configuration, Vercel frontend VITE_API_URL requirements, deployment verification examples, and detailed troubleshooting organized by component.
Frontend visual test mode, theme handling, and state management
frontend/src/App.tsx
Visual test mode query parameter routes to mock CampaignCard grid; theme handling centralized via data-theme attribute; campaign selection initialized from URL campaign param with invalid-ID tracking; keydown listener refactored with proper cleanup; campaign resolution logic updated for consistent selected-ID computation.
Frontend user action flows and UX improvements
frontend/src/App.tsx
Pledge, claim, soft-delete, and refund flows now use centralized getErrorMessage for error formatting, show success toasts on completion, ignore USER_CANCELLED, and clear pending state; wallet UX adds disconnect/account-switch toasts; campaign creation removes immediate navigation post-success; theme toggle button label updates based on themeMode.
Backend code formatting and CORS error handling
backend/src/index.ts, backend/src/services/sorobanRpc.ts, backend/package.json
Express middleware adds dedicated CORS error path returning 403 with FORBIDDEN status; string literals converted to single quotes across RPC service; generic error handler removes conditional details mapping; campaign response and error logging reformatted; backend dev script added for ts-node execution.
Repository tooling and formatting setup
package.json, test-results/.last-run.json, backend/test-results/.last-run.json
Prettier npm scripts (format, format:check) added to package.json with prettier added to devDependencies; test result status files updated to reflect failed run state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ritik4ever/stellar-goal-vault#334: Overlapping theme handling changes to frontend/src/App.tsx—this PR centralizes theme application on data-theme while that PR uses useLocalStorage for persistence.
  • ritik4ever/stellar-goal-vault#336: Both PRs modify backend/src/services/sorobanRpc.ts around Soroban refund validation and quote style formatting.

Poem

🐰 A carrot for the screenshots, pixel-perfect and bright!
The visual test suite now guards our CampaignCard's flight.
Deployment docs unfurl with care, from contract to cloud,
While the App refactors toast by toast, serving users proud.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check ❓ Inconclusive Several non-visual-regression changes are present: backend modifications, deployment documentation updates, and frontend App.tsx logic changes that appear unrelated to the core visual regression objective. Clarify whether backend/services updates, deployment docs, and App.tsx enhancements (theme handling, visual test mode, URL state management) are dependencies of the visual regression feature or should be separated into distinct pull requests.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Playwright visual regression' clearly and concisely summarizes the main change: adding Playwright visual regression testing, which is the primary focus of the pull request.
Linked Issues check ✅ Passed All acceptance criteria from issue #315 are met: CampaignCard visual tests render all four states, screenshots are configured for storage, pixel diff threshold is set to 0.5%, and CI artifact upload is configured in the workflow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@drips-wave

drips-wave Bot commented May 28, 2026

Copy link
Copy Markdown

@EmeditWeb Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/workflows/playwright-visual-regression.yml (1)

1-42: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Address security vulnerabilities in the visual regression workflow.

The workflow has the same security concerns flagged by static analysis:

  1. Unpinned actions: All three actions (checkout, setup-node, upload-artifact) use tag references instead of commit SHAs.
  2. Missing persist-credentials: false: The checkout step should disable credential persistence.
  3. No permissions block: Restrict permissions to the minimum required.
  4. Inconsistent dependency installation: Use npm ci for reproducible builds, not npm install.
🔒 Proposed security hardening
 name: Playwright Visual Regression
 
 on:
   pull_request:
     branches:
       - main
 
 jobs:
   visual-regression:
     name: Visual regression
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      actions: write  # for artifact upload
 
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
+        with:
+          persist-credentials: false
 
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
           node-version: 18
           cache: 'npm'
           cache-dependency-path: package-lock.json
 
       - name: Install dependencies
-        run: npm install
+        run: npm ci
 
       - name: Install Playwright browsers
         run: npx playwright install --with-deps
 
       - name: Run visual regression tests
         run: npm run test:visual
 
       - name: Upload visual regression artifacts on failure
         if: failure()
         uses: actions/upload-artifact@v4
         with:
           name: visual-regression-artifacts
           path: |
             playwright-report
             test-results
             e2e/screenshots
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/playwright-visual-regression.yml around lines 1 - 42,
Replace unpinned GitHub actions with commit SHA pins for actions/checkout,
actions/setup-node and actions/upload-artifact (do not rely on tag refs like
actions/checkout@v4), add persist-credentials: false to the checkout step
(symbol: persist-credentials), add a minimal permissions block (e.g.,
permissions: contents: read with any other granular perms required by the job)
to the workflow header (symbol: permissions), and change the dependency install
step to use npm ci instead of npm install (symbol: npm ci) to ensure
reproducible installs.
backend/src/index.ts (1)

624-626: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix backend/src/index.ts startup guard to be ESM-safe

backend/src/index.ts uses ESM (import.meta.url), and the backend dev script runs with ts-node --esm; in that mode require is undefined, so if (require.main === module) can throw a ReferenceError at module evaluation. Use an ESM “main module” check instead (you already have __filename and path in this file).

Suggested fix
-if (require.main === module) {
-  startServer();
-}
+const isMainModule = process.argv[1] && path.resolve(process.argv[1]) === __filename;
+if (isMainModule) {
+  startServer();
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/index.ts` around lines 624 - 626, The current CommonJS guard
using require.main causes a ReferenceError under ESM; replace it with an
ESM-safe "main module" check that uses the existing __filename (and path if
needed) to detect invocation and call startServer() only when the script is the
entrypoint—e.g. compare process.argv[1] to __filename (or compare
import.meta.url to the file URL built from __filename) and invoke startServer()
when they match so the check does not rely on require or throw under ts-node
--esm.
🧹 Nitpick comments (2)
frontend/package.json (1)

17-18: ⚡ Quick win

Align react / react-dom semver ranges (lockfile already pins both to 18.3.1).

frontend/package.json has react as ^18.2.0 and react-dom as ^18.3.1; frontend/package-lock.json currently installs both as 18.3.1, but aligning the ranges prevents future drift when the lockfile is regenerated.

Suggested fix
-    "react": "^18.2.0",
-    "react-dom": "^18.3.1",
+    "react": "^18.3.1",
+    "react-dom": "^18.3.1",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/package.json` around lines 17 - 18, Package.json lists "react":
"^18.2.0" while "react-dom": "^18.3.1"; update the semver ranges so they match
the installed lockfile version to prevent drift—for example change the "react"
entry to "^18.3.1" (so both "react" and "react-dom" use "^18.3.1") and save
package.json; this keeps the dependencies aligned with the package-lock
resolution.
frontend/src/components/campaignsTableUtils.ts (1)

57-67: ⚡ Quick win

applyFilters contract is inconsistent with implementation.

searchQuery is accepted but ignored, while the JSDoc says search is part of filter composition. Please either apply searchQuery in this function or remove the parameter and update docs/comments to avoid misleading callers.

Suggested alignment (remove unused search arg from this utility)
 export function applyFilters(
   campaigns: Campaign[],
   assetCode: string,
-  status: string,
-  searchQuery: string = '',
+  status: string,
 ): Campaign[] {
-  // Client-side filters (asset/status only, search is server-side)
+  // Client-side filters (asset/status only)
   return campaigns.filter((c) => {
     const matchesAsset = assetCode === '' || c.assetCode === assetCode;
     const matchesStatus = status === '' || c.progress.status === status;
     return matchesAsset && matchesStatus;
   });
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/campaignsTableUtils.ts` around lines 57 - 67, The
applyFilters function accepts a searchQuery parameter but never uses it; remove
the unused searchQuery parameter and any related JSDoc/comment that claims
search is handled here, update the applyFilters signature (remove searchQuery:
string = '') and its type annotations, and update all callers to stop passing a
searchQuery argument so the call sites match the new signature; keep the
existing client-side filtering logic (assetCode/status) in applyFilters and
ensure any server-side search behavior remains in the code that performs server
queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 65-84: The format-check job must be hardened: in the job
"format-check" replace unpinned actions (uses: actions/checkout@v4 and uses:
actions/setup-node@v4) with their pinned commit SHAs, add persist-credentials:
false under the Checkout code step, add a narrow permissions block at the job
level (e.g., permissions: contents: read) to restrict token scope, and change
the Install root dependencies step to use npm ci instead of npm install to match
other jobs and provide a reproducible clean install; apply these edits to the
steps named "Checkout code", "Set up Node.js", and "Install root dependencies"
respectively.

In `@backend/src/services/campaignController.ts`:
- Around line 11-13: Update the 400 error message to match the actual accepted
sort key names defined in VALID_SORT_FIELDS: replace "createdTime" with
"createdAt" in the JSON error response returned (the block that does return
res.status(400).json(...)). Ensure the message lists the exact keys from
VALID_SORT_FIELDS (createdAt, deadline, pledgedAmount, percentFunded) so clients
get the correct guidance.

In `@backend/test-results/.last-run.json`:
- Around line 1-4: Remove the committed volatile test-run artifact
.last-run.json from version control and stop tracking it, then add a rule to
ignore such ephemeral test state (e.g., add a pattern for .last-run.json or the
test-results directory) to .gitignore so it won’t be re-added; also ensure any
CI/build scripts don’t accidentally commit this file and remove it from the
current commit history/index (unstage/remove from repo) so future PRs aren’t
noisy.

In `@backend/tests/integration_test.ts`:
- Around line 348-365: The test sets a future deadline so the campaign remains
open; change the campaign creation in the failing test (call to
createTestCampaign) to use a past deadline (e.g., deadline: nowInSeconds() - 50
or nowInSeconds() - 1) so the campaign transitions to 'failed' immediately;
update the createTestCampaign invocation in backend/tests/integration_test.ts
(the test that uses nowInSeconds()) and keep the subsequent calls to
addTestPledge, getCampaignDetails and the assertions unchanged.
- Around line 67-77: The default test campaign fixture in createTestCampaign()
uses a description shorter than the backend minimum (20 chars); update the
default description string to meet the validation (e.g., make it >=20
characters) so tests using the fixture pass, keeping the function signature and
spread of overrides intact (ensure CREATOR_1, title, assetCode, targetAmount,
deadline and ...overrides remain unchanged).
- Around line 97-105: The default transactionHash produced by claimTestCampaign
is not a 64-character hex string required by backend validation; update
claimTestCampaign to supply a valid 64-char hex when txHash is omitted by
generating a 32-byte hex string (e.g. via Node's
crypto.randomBytes(32).toString('hex')) or by adding a helper like
generateValidTxHash() and using it as the fallback for transactionHash; ensure
you import/require crypto (or add the helper) and replace the current 'tx_' +
generateTestId('claim') fallback with the 64-char hex generator.
- Around line 570-572: The loop uses invalid contributor IDs like
`CONTRIBUTOR_${i}` so addTestPledge(campaignId, ...) fails validation; replace
those placeholders with real Stellar account IDs used in tests (e.g., generate
or obtain test keypairs and use their publicKey values or use existing test
fixture accounts) before calling addTestPledge, or call the helper that
creates/funds test accounts (e.g., createTestAccount/createFundedAccount) and
pass the returned account.publicKey; ensure the loop iterates over those valid
public keys so pledges succeed.

In `@DEPLOYMENT.md`:
- Line 78: The DB_PATH example is inconsistent with the Render Root Directory
being set to "backend"; update the example environment variable so it is
relative to that root (replace DB_PATH=backend/data/campaigns.db with
DB_PATH=data/campaigns.db) and apply the same fix for any other occurrences of
the redundant "backend/" prefix in the DEPLOYMENT.md examples or env snippets to
avoid creating nested backend/backend paths.

In `@frontend/src/App.tsx`:
- Around line 258-275: The early return when visualTestMode === 'campaign-card'
breaks React hook ordering; instead ensure all hooks in App.tsx run
unconditionally (so keep useState/useEffect/etc. at top) and only conditionally
render the visual test UI afterwards—e.g., run the hooks that define
visualCampaigns and visualSelectedCampaignId (and any other hooks) first, then
if visualTestMode === 'campaign-card' return or render the CampaignCard grid;
reference visualTestMode, visualCampaigns, visualSelectedCampaignId,
setVisualSelectedCampaignId and CampaignCard to locate and move the conditional
rendering below the hook calls or into a JSX branch so hook order is preserved.

In `@frontend/src/components/CampaignsTable.integration.test.tsx`:
- Around line 75-77: The teardown in CampaignsTable.integration.test.tsx only
calls vi.restoreAllMocks() but does not revert timers set by beforeEach's
vi.useFakeTimers(); add a call to vi.useRealTimers() (or
vi.runOnlyPendingTimers() then vi.useRealTimers() if needed) in the afterEach
block to ensure fake timers are restored to real timers and avoid cross-test
leakage; update the afterEach that currently references vi.restoreAllMocks() to
also invoke vi.useRealTimers(), targeting the same test file's
beforeEach/afterEach pair.

In `@frontend/src/components/CampaignsTable.tsx`:
- Line 2: The CampaignsTable component imports hooks but omits useEffect, which
is referenced in the component and causes a runtime error; update the import
line that currently reads "import { useMemo, useState } from 'react';" to also
include useEffect so the component (CampaignsTable) can call useEffect without
failing.

In `@frontend/src/components/CreateCampaignForm.test.tsx`:
- Around line 132-147: The "empty title" test in CreateCampaignForm.test.tsx
currently types 'My Test Campaign' into the title field (using the placeholder
/Stellar community design sprint/i), which invalidates the assertion expecting
'Campaign title is required'; to fix, update the test to leave the title empty
(remove the user.type call that fills the title) or explicitly clear the title
before submitting (e.g., target the same placeholder text and clear it) so that
when clicking the create button the form validation produces 'Campaign title is
required' as asserted.

In `@frontend/src/components/SearchInput.test.tsx`:
- Line 81: The test queries use getByPlaceholderText with the wrong placeholder
string ('Search campaigns..' two dots) causing mismatch with the expected
component placeholder ('Search campaigns...' three dots); update all
getByPlaceholderText calls that pass 'Search campaigns..' (e.g., the calls
assigning to the variable input and other similar queries) to use 'Search
campaigns...' so they match the component's actual placeholder text.

---

Outside diff comments:
In @.github/workflows/playwright-visual-regression.yml:
- Around line 1-42: Replace unpinned GitHub actions with commit SHA pins for
actions/checkout, actions/setup-node and actions/upload-artifact (do not rely on
tag refs like actions/checkout@v4), add persist-credentials: false to the
checkout step (symbol: persist-credentials), add a minimal permissions block
(e.g., permissions: contents: read with any other granular perms required by the
job) to the workflow header (symbol: permissions), and change the dependency
install step to use npm ci instead of npm install (symbol: npm ci) to ensure
reproducible installs.

In `@backend/src/index.ts`:
- Around line 624-626: The current CommonJS guard using require.main causes a
ReferenceError under ESM; replace it with an ESM-safe "main module" check that
uses the existing __filename (and path if needed) to detect invocation and call
startServer() only when the script is the entrypoint—e.g. compare
process.argv[1] to __filename (or compare import.meta.url to the file URL built
from __filename) and invoke startServer() when they match so the check does not
rely on require or throw under ts-node --esm.

---

Nitpick comments:
In `@frontend/package.json`:
- Around line 17-18: Package.json lists "react": "^18.2.0" while "react-dom":
"^18.3.1"; update the semver ranges so they match the installed lockfile version
to prevent drift—for example change the "react" entry to "^18.3.1" (so both
"react" and "react-dom" use "^18.3.1") and save package.json; this keeps the
dependencies aligned with the package-lock resolution.

In `@frontend/src/components/campaignsTableUtils.ts`:
- Around line 57-67: The applyFilters function accepts a searchQuery parameter
but never uses it; remove the unused searchQuery parameter and any related
JSDoc/comment that claims search is handled here, update the applyFilters
signature (remove searchQuery: string = '') and its type annotations, and update
all callers to stop passing a searchQuery argument so the call sites match the
new signature; keep the existing client-side filtering logic (assetCode/status)
in applyFilters and ensure any server-side search behavior remains in the code
that performs server queries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4330bae3-4eea-4547-8286-6f9dd93e9d4c

📥 Commits

Reviewing files that changed from the base of the PR and between 4fce8c0 and 1b1bad2.

⛔ Files ignored due to path filters (3)
  • backend/package-lock.json is excluded by !**/package-lock.json
  • frontend/package-lock.json is excluded by !**/package-lock.json
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (113)
  • .github/workflows/ci.yml
  • .github/workflows/playwright-visual-regression.yml
  • .prettierrc
  • .vscode/settings.json
  • DEPLOYMENT.md
  • backend/.eslintrc.json
  • backend/package.json
  • backend/src/api.test.ts
  • backend/src/config.ts
  • backend/src/index.test.ts
  • backend/src/index.ts
  • backend/src/logger.test.ts
  • backend/src/logger.ts
  • backend/src/middleware/requestLogging.ts
  • backend/src/pledgesEndpoint.test.ts
  • backend/src/services/__tests__/eventMetadata.test.ts
  • backend/src/services/campaignController.ts
  • backend/src/services/campaignStore.test.ts
  • backend/src/services/campaignStore.ts
  • backend/src/services/db.ts
  • backend/src/services/eventHistory.ts
  • backend/src/services/eventIndexer.ts
  • backend/src/services/logger.test.ts
  • backend/src/services/logger.ts
  • backend/src/services/openIssues.ts
  • backend/src/services/seedDeterministic.test.ts
  • backend/src/services/seedDeterministic.ts
  • backend/src/services/sorobanRpc.ts
  • backend/src/types/better-sqlite3.d.ts
  • backend/src/types/errors.ts
  • backend/src/validateEnv.ts
  • backend/src/validation/schemas.ts
  • backend/test-results/.last-run.json
  • backend/tests/integration_test.ts
  • backend/tests/utils.ts
  • backend/vitest.config.ts
  • contracts/test_snapshots/test/test_multi_token_campaign.1.json
  • contracts/test_snapshots/test/test_multi_token_refund.1.json
  • contracts/test_snapshots/test/test_unaccepted_token.1.json
  • contracts/test_snapshots/test/tests/test_claim_before_deadline.1.json
  • contracts/test_snapshots/test/tests/test_claim_creator_mismatch.1.json
  • contracts/test_snapshots/test/tests/test_claim_double_claim.1.json
  • contracts/test_snapshots/test/tests/test_claim_success.1.json
  • contracts/test_snapshots/test/tests/test_claim_underfunded.1.json
  • contracts/test_snapshots/test/tests/test_contribute_rejects_amount_over_target.1.json
  • contracts/test_snapshots/test/tests/test_create_campaign_rejects_excessive_duration.1.json
  • contracts/test_snapshots/test/tests/test_get_campaign_count_tracks_creates.1.json
  • e2e/campaign-card.visual.spec.ts
  • e2e/campaign-lifecycle.spec.ts
  • e2e/dashboard.ts
  • frontend/.eslintrc.json
  • frontend/Tippage.tsx
  • frontend/package.json
  • frontend/src/App.tsx
  • frontend/src/components/AddressAvatar.tsx
  • frontend/src/components/AssetFilterDropdown.tsx
  • frontend/src/components/CampaignCard.tsx
  • frontend/src/components/CampaignDetailPanel.test.tsx
  • frontend/src/components/CampaignDetailPanel.tsx
  • frontend/src/components/CampaignTimeline.tsx
  • frontend/src/components/CampaignsTable.a11y.test.tsx
  • frontend/src/components/CampaignsTable.integration.test.tsx
  • frontend/src/components/CampaignsTable.tsx
  • frontend/src/components/ContributorSummary.tsx
  • frontend/src/components/CopyButton.tsx
  • frontend/src/components/CreateCampaignForm.a11y.test.tsx
  • frontend/src/components/CreateCampaignForm.test.tsx
  • frontend/src/components/CreateCampaignForm.tsx
  • frontend/src/components/CreateCampaignForm.validation.test.tsx
  • frontend/src/components/CreatorAnalytics.tsx
  • frontend/src/components/EmptyState.tsx
  • frontend/src/components/ErrorBoundary.test.tsx
  • frontend/src/components/ErrorBoundary.tsx
  • frontend/src/components/FundedConfetti.test.tsx
  • frontend/src/components/FundedConfetti.tsx
  • frontend/src/components/IssueBacklog.tsx
  • frontend/src/components/KeyboardShortcutsOverlay.tsx
  • frontend/src/components/SearchInput.test.tsx
  • frontend/src/components/SearchInput.tsx
  • frontend/src/components/SortDropdown.tsx
  • frontend/src/components/ToastContainer.tsx
  • frontend/src/components/TransactionPreviewModal.tsx
  • frontend/src/components/WalletWidget.tsx
  • frontend/src/components/campaignsTableUtils.test.ts
  • frontend/src/components/campaignsTableUtils.ts
  • frontend/src/hooks/useDebounce.test.ts
  • frontend/src/hooks/useDebounce.ts
  • frontend/src/hooks/useFreighter.ts
  • frontend/src/hooks/useToast.test.ts
  • frontend/src/hooks/useToast.ts
  • frontend/src/lib/fundingCelebration.test.ts
  • frontend/src/lib/fundingCelebration.ts
  • frontend/src/lib/shortcuts.ts
  • frontend/src/main.tsx
  • frontend/src/services/api.ts
  • frontend/src/services/freighter.test.ts
  • frontend/src/services/freighter.ts
  • frontend/src/services/soroban.ts
  • frontend/src/test-setup.ts
  • frontend/src/types/campaign.ts
  • frontend/src/utils/exportCsv.ts
  • frontend/src/utils/validation.test.ts
  • frontend/src/utils/validation.ts
  • frontend/src/vite-env.d.ts
  • frontend/tsconfig.json
  • frontend/tsconfig.node.json
  • frontend/vite.config.ts
  • package.json
  • playwright-report/index.html
  • playwright.config.ts
  • playwright.visual.config.ts
  • release-please-config.json
  • test-results/.last-run.json

Comment thread .github/workflows/ci.yml
Comment on lines +65 to +84
format-check:
name: Prettier Format Check
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: 18
cache: "npm"
cache-dependency-path: package-lock.json

- name: Install root dependencies
run: npm install

- name: Run prettier format check
run: npm run format:check

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Address security vulnerabilities in the new format-check job.

The static analysis tool has identified several security concerns:

  1. Unpinned actions: Actions are referenced by tag (@v4) instead of commit SHA, which could allow malicious updates.
  2. Missing persist-credentials: false: The checkout step should disable credential persistence to prevent token leakage through artifacts.
  3. Excessive permissions: The job runs with default permissions; restrict using a permissions: block.
  4. Inconsistent dependency installation: Other jobs use npm ci (clean install), but this job uses npm install.
🔒 Proposed security hardening
   format-check:
     name: Prettier Format Check
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
 
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
+        with:
+          persist-credentials: false
 
       - name: Set up Node.js
         uses: actions/setup-node@v4
         with:
           node-version: 18
           cache: "npm"
           cache-dependency-path: package-lock.json
 
       - name: Install root dependencies
-        run: npm install
+        run: npm ci
 
       - name: Run prettier format check
         run: npm run format:check
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 70-71: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[warning] 65-84: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)


[error] 71-71: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 74-74: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 65 - 84, The format-check job must be
hardened: in the job "format-check" replace unpinned actions (uses:
actions/checkout@v4 and uses: actions/setup-node@v4) with their pinned commit
SHAs, add persist-credentials: false under the Checkout code step, add a narrow
permissions block at the job level (e.g., permissions: contents: read) to
restrict token scope, and change the Install root dependencies step to use npm
ci instead of npm install to match other jobs and provide a reproducible clean
install; apply these edits to the steps named "Checkout code", "Set up Node.js",
and "Install root dependencies" respectively.

Comment on lines +11 to +13
return res.status(400).json({
error:
'Invalid sort field or order. Use: createdTime, deadline, pledgedAmount, or percentFunded.',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix invalid sort field name in the 400 error message.

The response says createdTime, but the accepted key is createdAt (see VALID_SORT_FIELDS). This can mislead clients when correcting requests.

Suggested fix
-        'Invalid sort field or order. Use: createdTime, deadline, pledgedAmount, or percentFunded.',
+        'Invalid sort field or order. Use: createdAt, deadline, pledgedAmount, or percentFunded.',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return res.status(400).json({
error:
'Invalid sort field or order. Use: createdTime, deadline, pledgedAmount, or percentFunded.',
return res.status(400).json({
error:
'Invalid sort field or order. Use: createdAt, deadline, pledgedAmount, or percentFunded.',
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/campaignController.ts` around lines 11 - 13, Update the
400 error message to match the actual accepted sort key names defined in
VALID_SORT_FIELDS: replace "createdTime" with "createdAt" in the JSON error
response returned (the block that does return res.status(400).json(...)). Ensure
the message lists the exact keys from VALID_SORT_FIELDS (createdAt, deadline,
pledgedAmount, percentFunded) so clients get the correct guidance.

Comment on lines +1 to +4
{
"status": "failed",
"failedTests": []
} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not commit volatile test-run state artifacts.

This file stores ephemeral execution state, not source-of-truth test logic. Committing it will cause churn and misleading PR noise; remove it from git and ignore it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/test-results/.last-run.json` around lines 1 - 4, Remove the committed
volatile test-run artifact .last-run.json from version control and stop tracking
it, then add a rule to ignore such ephemeral test state (e.g., add a pattern for
.last-run.json or the test-results directory) to .gitignore so it won’t be
re-added; also ensure any CI/build scripts don’t accidentally commit this file
and remove it from the current commit history/index (unstage/remove from repo)
so future PRs aren’t noisy.

Comment on lines +67 to +77
async function createTestCampaign(overrides?: Partial<any>): Promise<AxiosResponse<any>> {
const baseTime = nowInSeconds();
return apiClient.post('/api/campaigns', {
creator: CREATOR_1,
title: 'Test Campaign',
description: 'A test campaign',
assetCode: 'USDC',
targetAmount: 1000,
deadline: baseTime + 86400, // 24 hours from now
...overrides,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Default campaign fixture violates backend validation constraints.

createTestCampaign() uses a default description shorter than the backend minimum (20 chars), so tests that rely on defaults can fail before exercising intended behavior.

Proposed fix
-    description: 'A test campaign',
+    description: 'A test campaign used for integration validation.',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function createTestCampaign(overrides?: Partial<any>): Promise<AxiosResponse<any>> {
const baseTime = nowInSeconds();
return apiClient.post('/api/campaigns', {
creator: CREATOR_1,
title: 'Test Campaign',
description: 'A test campaign',
assetCode: 'USDC',
targetAmount: 1000,
deadline: baseTime + 86400, // 24 hours from now
...overrides,
});
async function createTestCampaign(overrides?: Partial<any>): Promise<AxiosResponse<any>> {
const baseTime = nowInSeconds();
return apiClient.post('/api/campaigns', {
creator: CREATOR_1,
title: 'Test Campaign',
description: 'A test campaign used for integration validation.',
assetCode: 'USDC',
targetAmount: 1000,
deadline: baseTime + 86400, // 24 hours from now
...overrides,
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/integration_test.ts` around lines 67 - 77, The default test
campaign fixture in createTestCampaign() uses a description shorter than the
backend minimum (20 chars); update the default description string to meet the
validation (e.g., make it >=20 characters) so tests using the fixture pass,
keeping the function signature and spread of overrides intact (ensure CREATOR_1,
title, assetCode, targetAmount, deadline and ...overrides remain unchanged).

Comment on lines +97 to +105
async function claimTestCampaign(
campaignId: string,
creator: string,
txHash?: string,
): Promise<AxiosResponse<any>> {
return apiClient.post(`/api/campaigns/${campaignId}/claim`, {
creator,
transactionHash: txHash || 'tx_' + generateTestId('claim'),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Claim helper generates invalid transaction hashes.

The default transactionHash is not a 64-character hex string, but backend validation requires exactly that format.

Proposed fix
-    transactionHash: txHash || 'tx_' + generateTestId('claim'),
+    transactionHash: txHash || 'a'.repeat(64),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function claimTestCampaign(
campaignId: string,
creator: string,
txHash?: string,
): Promise<AxiosResponse<any>> {
return apiClient.post(`/api/campaigns/${campaignId}/claim`, {
creator,
transactionHash: txHash || 'tx_' + generateTestId('claim'),
});
async function claimTestCampaign(
campaignId: string,
creator: string,
txHash?: string,
): Promise<AxiosResponse<any>> {
return apiClient.post(`/api/campaigns/${campaignId}/claim`, {
creator,
transactionHash: txHash || 'a'.repeat(64),
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/integration_test.ts` around lines 97 - 105, The default
transactionHash produced by claimTestCampaign is not a 64-character hex string
required by backend validation; update claimTestCampaign to supply a valid
64-char hex when txHash is omitted by generating a 32-byte hex string (e.g. via
Node's crypto.randomBytes(32).toString('hex')) or by adding a helper like
generateValidTxHash() and using it as the fallback for transactionHash; ensure
you import/require crypto (or add the helper) and replace the current 'tx_' +
generateTestId('claim') fallback with the 64-char hex generator.

Comment thread frontend/src/App.tsx
Comment on lines +258 to +275
if (visualTestMode === 'campaign-card') {
return (
<main style={{ minHeight: '100vh', padding: 32, background: 'var(--bg)', color: 'var(--text-main)' }}>
<h1>CampaignCard visual regression</h1>
<div style={{ display: 'grid', gap: 24, marginTop: 24 }}>
{visualCampaigns.map((campaign) => (
<div key={campaign.id} data-testid={`campaign-card-${campaign.progress.status}`}>
<CampaignCard
campaign={campaign}
selectedCampaignId={visualSelectedCampaignId}
onSelect={setVisualSelectedCampaignId}
/>
</div>
))}
</div>
</main>
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Conditional early return breaks hook ordering guarantees.

Returning early for visualTestMode before later hooks means this component may execute a different number/order of hooks across renders.

Proposed direction
-  if (visualTestMode === 'campaign-card') {
-    return ( ... );
-  }
+  const isVisualCampaignCard = visualTestMode === 'campaign-card';
+  // keep all hooks at top-level in stable order
+  // then branch inside returned JSX or delegate to <VisualCampaignCardPage />
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` around lines 258 - 275, The early return when
visualTestMode === 'campaign-card' breaks React hook ordering; instead ensure
all hooks in App.tsx run unconditionally (so keep useState/useEffect/etc. at
top) and only conditionally render the visual test UI afterwards—e.g., run the
hooks that define visualCampaigns and visualSelectedCampaignId (and any other
hooks) first, then if visualTestMode === 'campaign-card' return or render the
CampaignCard grid; reference visualTestMode, visualCampaigns,
visualSelectedCampaignId, setVisualSelectedCampaignId and CampaignCard to locate
and move the conditional rendering below the hook calls or into a JSX branch so
hook order is preserved.

Comment on lines +75 to +77
afterEach(() => {
vi.restoreAllMocks();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify fake timers are always paired with real-timer teardown
rg -n -C2 "useFakeTimers|useRealTimers" --type ts

Repository: ritik4ever/stellar-goal-vault

Length of output: 2314


Restore real timers in CampaignsTable.integration.test.tsx teardown.

beforeEach calls vi.useFakeTimers(), but the shown teardown only calls vi.restoreAllMocks() and does not switch back to real timers, which can leak fake timers and make other tests flaky.

Suggested fix
  afterEach(() => {
+   vi.useRealTimers();
    vi.restoreAllMocks();
  });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
afterEach(() => {
vi.restoreAllMocks();
});
afterEach(() => {
vi.useRealTimers();
vi.restoreAllMocks();
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CampaignsTable.integration.test.tsx` around lines 75
- 77, The teardown in CampaignsTable.integration.test.tsx only calls
vi.restoreAllMocks() but does not revert timers set by beforeEach's
vi.useFakeTimers(); add a call to vi.useRealTimers() (or
vi.runOnlyPendingTimers() then vi.useRealTimers() if needed) in the afterEach
block to ensure fake timers are restored to real timers and avoid cross-test
leakage; update the afterEach that currently references vi.restoreAllMocks() to
also invoke vi.useRealTimers(), targeting the same test file's
beforeEach/afterEach pair.

import { SortDropdown, SortOption } from "./SortDropdown";
import { AddressAvatar } from "./AddressAvatar";
import { LayoutGrid } from 'lucide-react';
import { useMemo, useState } from 'react';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing useEffect import causes runtime failure.

Line 2 omits useEffect, but it is used on Line 65. This will break rendering with useEffect is not defined.

Suggested fix
-import { useMemo, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { useMemo, useState } from 'react';
import { useEffect, useMemo, useState } from 'react';
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CampaignsTable.tsx` at line 2, The CampaignsTable
component imports hooks but omits useEffect, which is referenced in the
component and causes a runtime error; update the import line that currently
reads "import { useMemo, useState } from 'react';" to also include useEffect so
the component (CampaignsTable) can call useEffect without failing.

Comment on lines +132 to 147
await user.type(
screen.getByPlaceholderText(/G\.\.\. creator public key/i),
`G${'A'.repeat(55)}`,
);
await user.type(
screen.getByPlaceholderText(/Stellar community design sprint/i),
'My Test Campaign',
);
await user.type(
screen.getByPlaceholderText(/Describe what the campaign funds/i),
'This campaign funds a real Soroban pledge flow for the MVP dashboard.',
);
await user.click(screen.getByText('USDC'));
await user.click(screen.getByRole('button', { name: /create campaign/i }));
expect(screen.getByText('Campaign title is required')).toBeInTheDocument();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

empty title test fills the title, invalidating its own assertion.

This test types 'My Test Campaign' (Lines 137–139) and later expects 'Campaign title is required' (Line 146). The assertion no longer matches the scenario and can fail incorrectly.

Suggested fix
-      await user.type(
-        screen.getByPlaceholderText(/Stellar community design sprint/i),
-        'My Test Campaign',
-      );
+      // keep title empty to validate required-title behavior
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await user.type(
screen.getByPlaceholderText(/G\.\.\. creator public key/i),
`G${'A'.repeat(55)}`,
);
await user.type(
screen.getByPlaceholderText(/Stellar community design sprint/i),
'My Test Campaign',
);
await user.type(
screen.getByPlaceholderText(/Describe what the campaign funds/i),
'This campaign funds a real Soroban pledge flow for the MVP dashboard.',
);
await user.click(screen.getByText('USDC'));
await user.click(screen.getByRole('button', { name: /create campaign/i }));
expect(screen.getByText('Campaign title is required')).toBeInTheDocument();
});
await user.type(
screen.getByPlaceholderText(/G\.\.\. creator public key/i),
`G${'A'.repeat(55)}`,
);
// keep title empty to validate required-title behavior
await user.type(
screen.getByPlaceholderText(/Describe what the campaign funds/i),
'This campaign funds a real Soroban pledge flow for the MVP dashboard.',
);
await user.click(screen.getByText('USDC'));
await user.click(screen.getByRole('button', { name: /create campaign/i }));
expect(screen.getByText('Campaign title is required')).toBeInTheDocument();
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CreateCampaignForm.test.tsx` around lines 132 - 147,
The "empty title" test in CreateCampaignForm.test.tsx currently types 'My Test
Campaign' into the title field (using the placeholder /Stellar community design
sprint/i), which invalidates the assertion expecting 'Campaign title is
required'; to fix, update the test to leave the title empty (remove the
user.type call that fills the title) or explicitly clear the title before
submitting (e.g., target the same placeholder text and clear it) so that when
clicking the create button the form validation produces 'Campaign title is
required' as asserted.

describe('Disabled State', () => {
it('should disable input when disabled prop is true', () => {
render(<SearchInput value="" onChange={mockOnChange} disabled={true} />);
const input = screen.getByPlaceholderText('Search campaigns..') as HTMLInputElement;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix placeholder expectation mismatch in test queries.

On Line 81, Line 91, Line 144, and Line 156, tests query 'Search campaigns..' (two dots), but the suite elsewhere expects 'Search campaigns...' (three dots). This inconsistency can cause false failures.

Proposed test fix
-const input = screen.getByPlaceholderText('Search campaigns..') as HTMLInputElement;
+const input = screen.getByPlaceholderText('Search campaigns...') as HTMLInputElement;

Also applies to: 91-91, 144-144, 156-156

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/SearchInput.test.tsx` at line 81, The test queries
use getByPlaceholderText with the wrong placeholder string ('Search campaigns..'
two dots) causing mismatch with the expected component placeholder ('Search
campaigns...' three dots); update all getByPlaceholderText calls that pass
'Search campaigns..' (e.g., the calls assigning to the variable input and other
similar queries) to use 'Search campaigns...' so they match the component's
actual placeholder text.

@ritik4ever

Copy link
Copy Markdown
Owner

@EmeditWeb please resolve conflicts

@EmeditWeb

Copy link
Copy Markdown
Contributor Author

@EmeditWeb please resolve conflicts

please merge

Vercel CI issues are from Authorization required to deploy issue not from me.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
frontend/src/App.tsx (2)

1-1: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add the missing hook/router imports.

This file now uses useRef, useNavigate, and useParams, but none of them are imported, so the app won't compile.

Suggested fix
-import { useEffect, useMemo, useState } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';
+import { useNavigate, useParams } from 'react-router-dom';

Also applies to: 116-117, 365-365

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` at line 1, The file is missing imports for hooks used
elsewhere—add useRef to the React import (alongside useEffect, useMemo,
useState) and import useNavigate and useParams from react-router-dom so
references to useRef, useNavigate, and useParams compile; update the top-level
import lines in App.tsx to include these symbols.

461-468: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pass the campaign id as nextSelectedId, not as the search query.

These handlers call refreshCampaigns(campaign.id), but the first parameter is searchQuery. After create/pledge/claim/refund, the UI will search for the id string instead of refreshing normally and keeping that campaign selected.

Suggested fix
-      await refreshCampaigns(campaign.id);
+      await refreshCampaigns('', campaign.id);

Apply the same change to the pledge, claim, and refund call sites.

Also applies to: 539-550, 597-598, 639-640

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` around lines 461 - 468, The handlers are currently
calling refreshCampaigns(campaign.id) which mistakenly passes the campaign id
into the searchQuery parameter; change calls to pass the id as the
nextSelectedId argument instead of searchQuery (i.e., call refreshCampaigns with
the search query left as its default/empty value and campaign.id as the
nextSelectedId). Update handleCreate and the pledge/claim/refund call sites (the
places that call refreshCampaigns after create/pledge/claim/refund) to use the
campaign id as nextSelectedId so the UI refreshes normally and keeps that
campaign selected.
frontend/src/components/CampaignsTable.tsx (1)

301-434: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

This JSX block is malformed after the mobile virtualization rewrite.

The branch mixes new <article> markup with leftover table cells/rows (</td>, </tr>, duplicate card markup), which matches the parser errors in the static analysis output. The component cannot build until this block is reduced to one valid mobile structure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CampaignsTable.tsx` around lines 301 - 434, The
mobile JSX block is broken by leftover table elements and duplicated card markup
after the virtualization rewrite; fix the mobile branch by removing stray table
fragments (</td>, </tr>, the tbody/table wrappers) and deleting the duplicate
non-virtualized "cards-only" mapping so only the virtualized mapping using
virtualizer.getVirtualItems(), article elements (with
ref={virtualizer.measureElement}), and the existing usage of filteredCampaigns,
selectedCampaignId, onSelect, AddressAvatar, getStatusLabel, and formatTimestamp
remain; ensure each article is properly closed and there are no duplicated card
lists.
backend/src/index.ts (1)

561-628: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

The error middleware is syntactically broken.

This handler closes at Line 572 and then repeats a second body afterward, leaving stray statements and the }, ) parse error Biome reported. As written, the file cannot parse and the generic error handler never registers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/index.ts` around lines 561 - 628, The error middleware is broken
by duplicated/stray code and an incorrectly closed handler; replace the two
fragments with one proper Express error-handling middleware using the
four-argument signature (err, req, res, next) registered via app.use, keep the
CORS branch for err.message === 'Not allowed by CORS' inside that function, then
compute statusCode and code (using AppError), build the ApiErrorResponse (using
RequestWithId for requestId), call logError with the same metadata, and finally
send res.status(statusCode).json(response); remove the duplicated blocks and the
stray closing tokens so the handler is a single coherent function using app.use,
AppError, ApiErrorResponse, RequestWithId, and logError.
backend/src/services/campaignStore.ts (1)

370-395: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Apply the same WHERE clause to the data query.

The filters only mutate baseQuery, but dataQuery still interpolates whereClause, which remains ''. That makes totalCount filtered while the returned campaign rows are not, so search/status filters and pagination drift apart.

Suggested fix
-  let whereClause = '';
+  const whereClause =
+    whereClauses.length > 0 ? ` WHERE ${whereClauses.join(' AND ')}` : '';

   if (!options?.includeDeleted) {
     whereClauses.push(`campaigns.deleted_at IS NULL`);
   }

-  if (whereClauses.length > 0) {
-    baseQuery += ` WHERE ` + whereClauses.join(' AND ');
-  }
+  baseQuery += whereClause;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/campaignStore.ts` around lines 370 - 395, The data query
is not using the same WHERE applied to baseQuery so countQuery is filtered but
dataQuery isn’t; update dataQuery to build from the same baseQuery (or append
the same whereClauses.join(' AND ')) rather than interpolating the stale
whereClause variable, and fix parameter handling so the data query uses a params
copy with limit/offset (e.g., create dataParams = params.slice(); if (paginate)
dataParams.push(limit, offset)) while leaving the original params for
countQuery; adjust usage of db.prepare(...).all(...) to pass dataParams for the
SELECT rows and params for the COUNT.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 48-53: Replace the tag-based action reference in the "Upload
backend audit artifact" step with a pinned commit SHA: change uses:
actions/upload-artifact@v4 to uses: actions/upload-artifact@<commit-sha> (use
the specific commit SHA from the actions/upload-artifact repository) and apply
the same pinning to other occurrences of actions/upload-artifact@v4 in the repo
(e.g., in the steps named similarly in playwright-visual-regression.yml,
frontend.yml, contracts-ci.yml, and playwright-e2e.yml) so the upload-artifact
action is referenced immutably.

In `@backend/src/index.ts`:
- Around line 84-97: The file references compression, apiKeyAuthMiddleware,
cacheMiddleware, and initRedisCache but never imports them; add top-level import
statements in backend/src/index.ts to pull compression from the compression
package and the apiKeyAuthMiddleware, cacheMiddleware, and initRedisCache from
your middleware/cache module (the same module that defines your cache and auth
utilities) so the app.use(...) calls (and the later references around lines
~658-665) compile correctly.
- Around line 294-300: The call to calculateProgress in the campaigns mapping
uses an outdated third argument (pledgeCounts[campaign.id]); update the call
site to the new signature by removing the third parameter so it reads
calculateProgress(campaign, undefined) (or just calculateProgress(campaign) if
appropriate), leaving the surrounding listCampaigns, campaigns, pledgeCounts,
and filterCampaignList usage unchanged; if pledgeCounts were intended to
influence progress, move that logic into calculateProgress or compute a derived
value before calling calculateProgress and pass it via the second optional "at"
parameter as needed.

In `@frontend/src/App.tsx`:
- Around line 256-265: The code uses SCROLL_KEY inside the useEffect (and also
at lines ~648-652) but SCROLL_KEY is not declared, causing a compile error and
broken scroll restore; define a single shared constant named SCROLL_KEY (for
example: const SCROLL_KEY = 'campaign-scroll-position') near the top of this
module or import it from your shared constants file, then replace any string
literals or missing references so both the useEffect that reads
sessionStorage.getItem(SCROLL_KEY) and the code that writes
sessionStorage.setItem(SCROLL_KEY, ...) use the same SCROLL_KEY symbol (ensure
the declaration is exported/imported if used across files).

In `@frontend/src/components/CampaignsTable.tsx`:
- Around line 101-107: The component uses useMediaQuery and useWindowVirtualizer
but never imports them; add the missing imports at the top of the file so the
component compiles. Specifically, import useMediaQuery (e.g., from `@mui/material`
or your project's UI lib) and import useWindowVirtualizer from the
virtualization library you use (e.g., `@tanstack/react-virtual`), then ensure the
symbols used in CampaignsTable (useMediaQuery and useWindowVirtualizer) refer to
those imports.

---

Outside diff comments:
In `@backend/src/index.ts`:
- Around line 561-628: The error middleware is broken by duplicated/stray code
and an incorrectly closed handler; replace the two fragments with one proper
Express error-handling middleware using the four-argument signature (err, req,
res, next) registered via app.use, keep the CORS branch for err.message === 'Not
allowed by CORS' inside that function, then compute statusCode and code (using
AppError), build the ApiErrorResponse (using RequestWithId for requestId), call
logError with the same metadata, and finally send
res.status(statusCode).json(response); remove the duplicated blocks and the
stray closing tokens so the handler is a single coherent function using app.use,
AppError, ApiErrorResponse, RequestWithId, and logError.

In `@backend/src/services/campaignStore.ts`:
- Around line 370-395: The data query is not using the same WHERE applied to
baseQuery so countQuery is filtered but dataQuery isn’t; update dataQuery to
build from the same baseQuery (or append the same whereClauses.join(' AND '))
rather than interpolating the stale whereClause variable, and fix parameter
handling so the data query uses a params copy with limit/offset (e.g., create
dataParams = params.slice(); if (paginate) dataParams.push(limit, offset)) while
leaving the original params for countQuery; adjust usage of
db.prepare(...).all(...) to pass dataParams for the SELECT rows and params for
the COUNT.

In `@frontend/src/App.tsx`:
- Line 1: The file is missing imports for hooks used elsewhere—add useRef to the
React import (alongside useEffect, useMemo, useState) and import useNavigate and
useParams from react-router-dom so references to useRef, useNavigate, and
useParams compile; update the top-level import lines in App.tsx to include these
symbols.
- Around line 461-468: The handlers are currently calling
refreshCampaigns(campaign.id) which mistakenly passes the campaign id into the
searchQuery parameter; change calls to pass the id as the nextSelectedId
argument instead of searchQuery (i.e., call refreshCampaigns with the search
query left as its default/empty value and campaign.id as the nextSelectedId).
Update handleCreate and the pledge/claim/refund call sites (the places that call
refreshCampaigns after create/pledge/claim/refund) to use the campaign id as
nextSelectedId so the UI refreshes normally and keeps that campaign selected.

In `@frontend/src/components/CampaignsTable.tsx`:
- Around line 301-434: The mobile JSX block is broken by leftover table elements
and duplicated card markup after the virtualization rewrite; fix the mobile
branch by removing stray table fragments (</td>, </tr>, the tbody/table
wrappers) and deleting the duplicate non-virtualized "cards-only" mapping so
only the virtualized mapping using virtualizer.getVirtualItems(), article
elements (with ref={virtualizer.measureElement}), and the existing usage of
filteredCampaigns, selectedCampaignId, onSelect, AddressAvatar, getStatusLabel,
and formatTimestamp remain; ensure each article is properly closed and there are
no duplicated card lists.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a52f2a74-91af-4fe6-934b-1850b6b8f696

📥 Commits

Reviewing files that changed from the base of the PR and between 1b1bad2 and b720817.

📒 Files selected for processing (10)
  • .github/workflows/ci.yml
  • DEPLOYMENT.md
  • backend/src/index.ts
  • backend/src/services/campaignStore.ts
  • backend/src/services/eventHistory.ts
  • e2e/campaign-lifecycle.spec.ts
  • e2e/dashboard.ts
  • frontend/src/App.tsx
  • frontend/src/components/CampaignsTable.tsx
  • frontend/src/utils/validation.ts
✅ Files skipped from review due to trivial changes (2)
  • e2e/dashboard.ts
  • frontend/src/utils/validation.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • backend/src/services/eventHistory.ts
  • e2e/campaign-lifecycle.spec.ts
  • DEPLOYMENT.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
frontend/src/App.tsx (2)

1-1: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add the missing hook/router imports.

This file now uses useRef, useNavigate, and useParams, but none of them are imported, so the app won't compile.

Suggested fix
-import { useEffect, useMemo, useState } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';
+import { useNavigate, useParams } from 'react-router-dom';

Also applies to: 116-117, 365-365

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` at line 1, The file is missing imports for hooks used
elsewhere—add useRef to the React import (alongside useEffect, useMemo,
useState) and import useNavigate and useParams from react-router-dom so
references to useRef, useNavigate, and useParams compile; update the top-level
import lines in App.tsx to include these symbols.

461-468: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pass the campaign id as nextSelectedId, not as the search query.

These handlers call refreshCampaigns(campaign.id), but the first parameter is searchQuery. After create/pledge/claim/refund, the UI will search for the id string instead of refreshing normally and keeping that campaign selected.

Suggested fix
-      await refreshCampaigns(campaign.id);
+      await refreshCampaigns('', campaign.id);

Apply the same change to the pledge, claim, and refund call sites.

Also applies to: 539-550, 597-598, 639-640

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` around lines 461 - 468, The handlers are currently
calling refreshCampaigns(campaign.id) which mistakenly passes the campaign id
into the searchQuery parameter; change calls to pass the id as the
nextSelectedId argument instead of searchQuery (i.e., call refreshCampaigns with
the search query left as its default/empty value and campaign.id as the
nextSelectedId). Update handleCreate and the pledge/claim/refund call sites (the
places that call refreshCampaigns after create/pledge/claim/refund) to use the
campaign id as nextSelectedId so the UI refreshes normally and keeps that
campaign selected.
frontend/src/components/CampaignsTable.tsx (1)

301-434: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

This JSX block is malformed after the mobile virtualization rewrite.

The branch mixes new <article> markup with leftover table cells/rows (</td>, </tr>, duplicate card markup), which matches the parser errors in the static analysis output. The component cannot build until this block is reduced to one valid mobile structure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CampaignsTable.tsx` around lines 301 - 434, The
mobile JSX block is broken by leftover table elements and duplicated card markup
after the virtualization rewrite; fix the mobile branch by removing stray table
fragments (</td>, </tr>, the tbody/table wrappers) and deleting the duplicate
non-virtualized "cards-only" mapping so only the virtualized mapping using
virtualizer.getVirtualItems(), article elements (with
ref={virtualizer.measureElement}), and the existing usage of filteredCampaigns,
selectedCampaignId, onSelect, AddressAvatar, getStatusLabel, and formatTimestamp
remain; ensure each article is properly closed and there are no duplicated card
lists.
backend/src/index.ts (1)

561-628: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

The error middleware is syntactically broken.

This handler closes at Line 572 and then repeats a second body afterward, leaving stray statements and the }, ) parse error Biome reported. As written, the file cannot parse and the generic error handler never registers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/index.ts` around lines 561 - 628, The error middleware is broken
by duplicated/stray code and an incorrectly closed handler; replace the two
fragments with one proper Express error-handling middleware using the
four-argument signature (err, req, res, next) registered via app.use, keep the
CORS branch for err.message === 'Not allowed by CORS' inside that function, then
compute statusCode and code (using AppError), build the ApiErrorResponse (using
RequestWithId for requestId), call logError with the same metadata, and finally
send res.status(statusCode).json(response); remove the duplicated blocks and the
stray closing tokens so the handler is a single coherent function using app.use,
AppError, ApiErrorResponse, RequestWithId, and logError.
backend/src/services/campaignStore.ts (1)

370-395: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Apply the same WHERE clause to the data query.

The filters only mutate baseQuery, but dataQuery still interpolates whereClause, which remains ''. That makes totalCount filtered while the returned campaign rows are not, so search/status filters and pagination drift apart.

Suggested fix
-  let whereClause = '';
+  const whereClause =
+    whereClauses.length > 0 ? ` WHERE ${whereClauses.join(' AND ')}` : '';

   if (!options?.includeDeleted) {
     whereClauses.push(`campaigns.deleted_at IS NULL`);
   }

-  if (whereClauses.length > 0) {
-    baseQuery += ` WHERE ` + whereClauses.join(' AND ');
-  }
+  baseQuery += whereClause;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/services/campaignStore.ts` around lines 370 - 395, The data query
is not using the same WHERE applied to baseQuery so countQuery is filtered but
dataQuery isn’t; update dataQuery to build from the same baseQuery (or append
the same whereClauses.join(' AND ')) rather than interpolating the stale
whereClause variable, and fix parameter handling so the data query uses a params
copy with limit/offset (e.g., create dataParams = params.slice(); if (paginate)
dataParams.push(limit, offset)) while leaving the original params for
countQuery; adjust usage of db.prepare(...).all(...) to pass dataParams for the
SELECT rows and params for the COUNT.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 48-53: Replace the tag-based action reference in the "Upload
backend audit artifact" step with a pinned commit SHA: change uses:
actions/upload-artifact@v4 to uses: actions/upload-artifact@<commit-sha> (use
the specific commit SHA from the actions/upload-artifact repository) and apply
the same pinning to other occurrences of actions/upload-artifact@v4 in the repo
(e.g., in the steps named similarly in playwright-visual-regression.yml,
frontend.yml, contracts-ci.yml, and playwright-e2e.yml) so the upload-artifact
action is referenced immutably.

In `@backend/src/index.ts`:
- Around line 84-97: The file references compression, apiKeyAuthMiddleware,
cacheMiddleware, and initRedisCache but never imports them; add top-level import
statements in backend/src/index.ts to pull compression from the compression
package and the apiKeyAuthMiddleware, cacheMiddleware, and initRedisCache from
your middleware/cache module (the same module that defines your cache and auth
utilities) so the app.use(...) calls (and the later references around lines
~658-665) compile correctly.
- Around line 294-300: The call to calculateProgress in the campaigns mapping
uses an outdated third argument (pledgeCounts[campaign.id]); update the call
site to the new signature by removing the third parameter so it reads
calculateProgress(campaign, undefined) (or just calculateProgress(campaign) if
appropriate), leaving the surrounding listCampaigns, campaigns, pledgeCounts,
and filterCampaignList usage unchanged; if pledgeCounts were intended to
influence progress, move that logic into calculateProgress or compute a derived
value before calling calculateProgress and pass it via the second optional "at"
parameter as needed.

In `@frontend/src/App.tsx`:
- Around line 256-265: The code uses SCROLL_KEY inside the useEffect (and also
at lines ~648-652) but SCROLL_KEY is not declared, causing a compile error and
broken scroll restore; define a single shared constant named SCROLL_KEY (for
example: const SCROLL_KEY = 'campaign-scroll-position') near the top of this
module or import it from your shared constants file, then replace any string
literals or missing references so both the useEffect that reads
sessionStorage.getItem(SCROLL_KEY) and the code that writes
sessionStorage.setItem(SCROLL_KEY, ...) use the same SCROLL_KEY symbol (ensure
the declaration is exported/imported if used across files).

In `@frontend/src/components/CampaignsTable.tsx`:
- Around line 101-107: The component uses useMediaQuery and useWindowVirtualizer
but never imports them; add the missing imports at the top of the file so the
component compiles. Specifically, import useMediaQuery (e.g., from `@mui/material`
or your project's UI lib) and import useWindowVirtualizer from the
virtualization library you use (e.g., `@tanstack/react-virtual`), then ensure the
symbols used in CampaignsTable (useMediaQuery and useWindowVirtualizer) refer to
those imports.

---

Outside diff comments:
In `@backend/src/index.ts`:
- Around line 561-628: The error middleware is broken by duplicated/stray code
and an incorrectly closed handler; replace the two fragments with one proper
Express error-handling middleware using the four-argument signature (err, req,
res, next) registered via app.use, keep the CORS branch for err.message === 'Not
allowed by CORS' inside that function, then compute statusCode and code (using
AppError), build the ApiErrorResponse (using RequestWithId for requestId), call
logError with the same metadata, and finally send
res.status(statusCode).json(response); remove the duplicated blocks and the
stray closing tokens so the handler is a single coherent function using app.use,
AppError, ApiErrorResponse, RequestWithId, and logError.

In `@backend/src/services/campaignStore.ts`:
- Around line 370-395: The data query is not using the same WHERE applied to
baseQuery so countQuery is filtered but dataQuery isn’t; update dataQuery to
build from the same baseQuery (or append the same whereClauses.join(' AND '))
rather than interpolating the stale whereClause variable, and fix parameter
handling so the data query uses a params copy with limit/offset (e.g., create
dataParams = params.slice(); if (paginate) dataParams.push(limit, offset)) while
leaving the original params for countQuery; adjust usage of
db.prepare(...).all(...) to pass dataParams for the SELECT rows and params for
the COUNT.

In `@frontend/src/App.tsx`:
- Line 1: The file is missing imports for hooks used elsewhere—add useRef to the
React import (alongside useEffect, useMemo, useState) and import useNavigate and
useParams from react-router-dom so references to useRef, useNavigate, and
useParams compile; update the top-level import lines in App.tsx to include these
symbols.
- Around line 461-468: The handlers are currently calling
refreshCampaigns(campaign.id) which mistakenly passes the campaign id into the
searchQuery parameter; change calls to pass the id as the nextSelectedId
argument instead of searchQuery (i.e., call refreshCampaigns with the search
query left as its default/empty value and campaign.id as the nextSelectedId).
Update handleCreate and the pledge/claim/refund call sites (the places that call
refreshCampaigns after create/pledge/claim/refund) to use the campaign id as
nextSelectedId so the UI refreshes normally and keeps that campaign selected.

In `@frontend/src/components/CampaignsTable.tsx`:
- Around line 301-434: The mobile JSX block is broken by leftover table elements
and duplicated card markup after the virtualization rewrite; fix the mobile
branch by removing stray table fragments (</td>, </tr>, the tbody/table
wrappers) and deleting the duplicate non-virtualized "cards-only" mapping so
only the virtualized mapping using virtualizer.getVirtualItems(), article
elements (with ref={virtualizer.measureElement}), and the existing usage of
filteredCampaigns, selectedCampaignId, onSelect, AddressAvatar, getStatusLabel,
and formatTimestamp remain; ensure each article is properly closed and there are
no duplicated card lists.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a52f2a74-91af-4fe6-934b-1850b6b8f696

📥 Commits

Reviewing files that changed from the base of the PR and between 1b1bad2 and b720817.

📒 Files selected for processing (10)
  • .github/workflows/ci.yml
  • DEPLOYMENT.md
  • backend/src/index.ts
  • backend/src/services/campaignStore.ts
  • backend/src/services/eventHistory.ts
  • e2e/campaign-lifecycle.spec.ts
  • e2e/dashboard.ts
  • frontend/src/App.tsx
  • frontend/src/components/CampaignsTable.tsx
  • frontend/src/utils/validation.ts
✅ Files skipped from review due to trivial changes (2)
  • e2e/dashboard.ts
  • frontend/src/utils/validation.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • backend/src/services/eventHistory.ts
  • e2e/campaign-lifecycle.spec.ts
  • DEPLOYMENT.md
🛑 Comments failed to post (5)
.github/workflows/ci.yml (1)

48-53: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'uses:\s+[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+@v[0-9]+' .github/workflows/ci.yml

Repository: ritik4ever/stellar-goal-vault

Length of output: 391


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find any SHA-pinned GitHub Actions in workflows (40-hex)
rg -n --hidden --no-ignore -S "uses:\s+[^/\s]+/[^@\s]+@[0-9a-f]{40}\b" .github/workflows || true

# 2) Find any SHA-pinned GitHub Actions in the whole repo (broader)
rg -n --hidden --no-ignore -S "uses:\s+[^/\s]+/[^@\s]+@[0-9a-f]{40}\b" . || true

# 3) Find all upload-artifact uses
rg -n --hidden --no-ignore -S "uses:\s+actions/upload-artifact@" .github/workflows || true

Repository: ritik4ever/stellar-goal-vault

Length of output: 638


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find any SHA-pinned GitHub Actions in workflows (40-hex)
rg -n --hidden --no-ignore -S "uses:\s+[^/\s]+/[^@\s]+@[0-9a-f]{40}\b" .github/workflows || true

# 2) Find any SHA-pinned GitHub Actions in the whole repo (broader)
rg -n --hidden --no-ignore -S "uses:\s+[^/\s]+/[^@\s]+@[0-9a-f]{40}\b" . || true

# 3) Find all upload-artifact uses
rg -n --hidden --no-ignore -S "uses:\s+actions/upload-artifact@" .github/workflows || true

Repository: ritik4ever/stellar-goal-vault

Length of output: 638


Pin actions/upload-artifact to a commit SHA.

This workflow uses the tag-based action reference actions/upload-artifact@v4. The same unpinned @v4 reference is also used in multiple other workflows (.github/workflows/playwright-visual-regression.yml, .github/workflows/frontend.yml, .github/workflows/contracts-ci.yml, .github/workflows/playwright-e2e.yml). Pin to the exact commit SHA to keep the supply-chain boundary immutable.

- name: Upload backend audit artifact
  if: always()
  uses: actions/upload-artifact@v4
  with:
    name: backend-npm-audit
    path: /tmp/backend-audit.json
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 11-53: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)


[error] 50-50: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 48 - 53, Replace the tag-based action
reference in the "Upload backend audit artifact" step with a pinned commit SHA:
change uses: actions/upload-artifact@v4 to uses:
actions/upload-artifact@<commit-sha> (use the specific commit SHA from the
actions/upload-artifact repository) and apply the same pinning to other
occurrences of actions/upload-artifact@v4 in the repo (e.g., in the steps named
similarly in playwright-visual-regression.yml, frontend.yml, contracts-ci.yml,
and playwright-e2e.yml) so the upload-artifact action is referenced immutably.
backend/src/index.ts (2)

84-97: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Import the new middleware/cache symbols before using them.

compression, apiKeyAuthMiddleware, cacheMiddleware, and initRedisCache are referenced here but never imported, so this file will fail to compile.

Also applies to: 658-665

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/index.ts` around lines 84 - 97, The file references compression,
apiKeyAuthMiddleware, cacheMiddleware, and initRedisCache but never imports
them; add top-level import statements in backend/src/index.ts to pull
compression from the compression package and the apiKeyAuthMiddleware,
cacheMiddleware, and initRedisCache from your middleware/cache module (the same
module that defines your cache and auth utilities) so the app.use(...) calls
(and the later references around lines ~658-665) compile correctly.

294-300: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Update this call site to the new calculateProgress signature.

calculateProgress now takes only (campaign, at?), but this route still passes a third pledgeCounts[...] argument. The backend won't type-check until this contract is updated.

Suggested fix
-      progress: calculateProgress(campaign, undefined, pledgeCounts[campaign.id]),
+      progress: calculateProgress(campaign),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const { campaigns, totalCount, pledgeCounts } = listCampaigns(listOptions);

  const data = filterCampaignList(
    campaigns.map((campaign) => ({
      ...campaign,
      progress: calculateProgress(campaign),
    })),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/index.ts` around lines 294 - 300, The call to calculateProgress
in the campaigns mapping uses an outdated third argument
(pledgeCounts[campaign.id]); update the call site to the new signature by
removing the third parameter so it reads calculateProgress(campaign, undefined)
(or just calculateProgress(campaign) if appropriate), leaving the surrounding
listCampaigns, campaigns, pledgeCounts, and filterCampaignList usage unchanged;
if pledgeCounts were intended to influence progress, move that logic into
calculateProgress or compute a derived value before calling calculateProgress
and pass it via the second optional "at" parameter as needed.
frontend/src/App.tsx (1)

256-265: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Define SCROLL_KEY before using it.

SCROLL_KEY is read and written here, but there is no declaration in the file. That is an immediate compile failure and also breaks the scroll-restore path.

Also applies to: 648-652

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/App.tsx` around lines 256 - 265, The code uses SCROLL_KEY inside
the useEffect (and also at lines ~648-652) but SCROLL_KEY is not declared,
causing a compile error and broken scroll restore; define a single shared
constant named SCROLL_KEY (for example: const SCROLL_KEY =
'campaign-scroll-position') near the top of this module or import it from your
shared constants file, then replace any string literals or missing references so
both the useEffect that reads sessionStorage.getItem(SCROLL_KEY) and the code
that writes sessionStorage.setItem(SCROLL_KEY, ...) use the same SCROLL_KEY
symbol (ensure the declaration is exported/imported if used across files).
frontend/src/components/CampaignsTable.tsx (1)

101-107: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Import the virtualization hooks before using them.

useMediaQuery and useWindowVirtualizer are introduced here but never imported, so the component will not compile even after the JSX is fixed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/CampaignsTable.tsx` around lines 101 - 107, The
component uses useMediaQuery and useWindowVirtualizer but never imports them;
add the missing imports at the top of the file so the component compiles.
Specifically, import useMediaQuery (e.g., from `@mui/material` or your project's
UI lib) and import useWindowVirtualizer from the virtualization library you use
(e.g., `@tanstack/react-virtual`), then ensure the symbols used in CampaignsTable
(useMediaQuery and useWindowVirtualizer) refer to those imports.

@ritik4ever
ritik4ever merged commit 977984d into ritik4ever:main Jun 2, 2026
1 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Playwright visual regression test for CampaignCard component

2 participants